Hi @nuclearsandwich, just a curiosity: how does this interact with the Gazebo binaries that are released on http://packages.osrfoundation.org and that transitively link (via DART) to the Ubuntu upstream fcl? (Related comment in another issue: #26527 (comment)).

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/release-package-requiring-custom-version-of-system-dependency/18200/7

Hmm based on the discussion above, maybe I shouldn't merge this. I'll wait for followups and someone else who knows more can merge.

To add some context, this is the result of the discussion at: #27789

I don't want to put this discussion on hold for too long but I do want to actually examine the decisions leading up to releasing fcl in noetic so that we can either adhere to our own guidelines better in the future or spell out what makes fcl different in this case. @tfoote's comment here should be set down as review policy if it isn't already.

I think there's some case to be made for "well we let it in for Noetic and so ROS 2 distributions built on Focal should be able to use the same fcl dependency in ROS Noetic and ROS Foxy and Galactic (which would be next targets after this Rolling release) and when 22.04 becomes available for which 0.6.1 is the likely upstream version as it is already part of Debian unstable we would drop the in-distribution fcl package.

I'd like to bump this conversation since getting fcl-0.6 into Rolling is important if we're going to do a release of the RMF packages into Rolling. While the RMF packages can be compiled against either fcl-0.5 or fcl-0.6, we have come across some critical errors in fcl-0.5 where its continuous collision detection may give false negatives for known collisions. Those issues are fixed in fcl-0.6, and I think it's unlikely that we'll see the fixes backported since the code of 0.6 diverges so dramatically from 0.5.

I would feel very uncomfortable releasing RMF on top of a package version that has these known issues, since the false negatives can cause permanent deadlock between robots that are trying to navigate around each other (effectively they don't "see" each other from the perspective of the planner, so they'll just keep trying to plan their paths through each other).

It's true that we could pull the fcl-0.6 source code into our own codebase and statically link against it, but I expect this issue is likely to impact other users, so it would be unfortunate if the whole ecosystem had to use this approach to ensure correct continuous collision detection.

To address @traversaro's concerns, I'm pretty sure there shouldn't be any linking issues since fcl-0.6 has moved everything into a template implementation. I believe a lucky byproduct of that change is that all of the symbols in fcl-0.6 will be generated with different names from fcl-0.5 even if none of the namespaces or class (now template) names get changed.

In the short run, this one will definitely conflict so can't be merged as is without a refactor or isolation process.

What should we do about fcl in noetic, which has already been released. Should fcl 0.6 be removed there?

I think for Noetic it's too baked in now to remove with many packages relying on it. Luckily it might not conflict too much due to the template changes mentioned above which is probably why we haven't gotten too many user reports of issues. We also may not get reports of issues when we break other system installations.

...

On more research I find that there are actually issues reported with conflicts in noetic: #26706 (comment)

The resolution for that was to just tell people to use the newer version which is exactly the conflicts that we should be avoiding.

There's more backstory on conflicts over here too: #26527

In general the more things that we override the tighter the box we paint ourselves into.

@tfoote I understand the concerns you've outlined, and I can see how a strict stance on this is justified.

Ordinarily I would agree with you about backporting the necessary fixes to fcl-0.5, but the truth is the change from 0.5 -> 0.6 is almost a complete rewrite, and we don't know which specific changes fixed the issues we were seeing or even what the root cause of the bug was in fcl-0.5. All we know is that our unit tests were giving false negatives in some circumstances when using fcl-0.5, while simply switching to fcl-0.6 allowed the tests to pass.

Given that 0.5 isn't a version that anyone seems to want to use (..as far as I can tell, people only use it because it happens to be the version available in their distro..), I find it hard to justify the effort it would take to track down the bugs and fix them for a patch release of 0.5.

I guess we'll take the approach of stashing the fcl-0.6 source code into our package that depends on it and statically linking it. Our package's API doesn't expose any FCL, so there shouldn't be any issue for downstream users.

To address @traversaro's concerns, I'm pretty sure there shouldn't be any linking issues since fcl-0.6 has moved everything into a template implementation. I believe a lucky byproduct of that change is that all of the symbols in fcl-0.6 will be generated with different names from fcl-0.5 even if none of the namespaces or class (now template) names get changed.

To clarify, I am happy with whatever solution OR reaches for this problem. My original comment was just to make sure that everyone involved was aligned w.r.t. to this problem, as in the past I had similar problem with other packages from the ros2 distribution that were shadowing the system ones (in particular urdfdom, see robotology-legacy/gym-ignition#118). If consensus is reached that the pros of packaging fcl 0.6 on rolling are more then the cons, I would be happy with that.

On an unrelated note, I think that to avoid similar problems in the future (Ubuntu 22.04, for example) it would be nice to have a new release of fcl in 2021 with the latest bug fixes, see flexible-collision-library/fcl#532 . Not sure who is the right person to ping on this. I guess @j-rivero is the right person to get the release in Debian sid once a new release is done, but before that a new release needs to be done at the fcl level.

Closing this PR since we're not going to release fcl into Rolling. The system version of fcl should be used by ROS packages.